Better parsing for global ostree options

 * Specifying global options after the command for a more natural:
   # ostree commit --repo=/path/to/repo ...
 * Support asking for --help without --repo
   # ostree commit --help
 * Support short form of -h
 * Support specifying --repo without equals sign
   # ostree --repo /path/to/repo commit ...
 * Support global --help and -h
   # ostree --help
 * Ditto for ostree admin sub commands
 * Removed some leaky code

https://bugzilla.gnome.org/show_bug.cgi?id=705903

diff --git a/src/ostree/ot-builtin-admin.c b/src/ostree/ot-builtin-admin.c
index e6aaee68077a6e1c74b2186586898ea5080092ce..16d633e4d57b2b1389241e30fdfdb74aafaa07a9 100644 (file)
--- a/src/ostree/ot-builtin-admin.c
+++ b/src/ostree/ot-builtin-admin.c
@@ -54,19 +54,95 @@ ostree_builtin_admin (int argc, char **argv, GFile *repo_path, GCancellable *can
 {
   gboolean ret = FALSE;
   const char *opt_sysroot = "/";
-  const char *subcommand_name;
+  const char *subcommand_name = NULL;
   OstreeAdminCommand *subcommand;
-  int subcmd_argc;
   gs_unref_object GFile *sysroot = NULL;
-  char **subcmd_argv = NULL;
+  gboolean want_help = FALSE;
+  int in, out, i;
+  gboolean skip;
 
-  if (argc > 1 && g_str_has_prefix (argv[1], "--sysroot="))
+  /*
+   * Parse the global options. We rearrange the options as
+   * necessary, in order to pass relevant options through
+   * to the commands, but also have them take effect globally.
+   */
+
+  for (in = 1, out = 1; in < argc; in++, out++)
     {
-      opt_sysroot = argv[1] + strlen ("--sysroot=");
-      argc--;
-      argv++;
+      /* The non-option is the command, take it out of the arguments */
+      if (argv[in][0] != '-')
+        {
+          skip = (subcommand_name == NULL);
+          if (subcommand_name == NULL)
+            subcommand_name = argv[in];
+        }
+
+      /* The global long options */
+      else if (argv[in][1] == '-')
+        {
+          skip = FALSE;
+
+          if (g_str_equal (argv[in], "--"))
+            {
+              break;
+            }
+          else if (g_str_equal (argv[in], "--help"))
+            {
+              want_help = TRUE;
+            }
+          else if (g_str_equal (argv[in], "--sysroot") && in + 1 < argc)
+            {
+              opt_sysroot = argv[in + 1];
+              skip = TRUE;
+              in++;
+            }
+          else if (g_str_has_prefix (argv[in], "--sysroot="))
+            {
+              opt_sysroot = argv[in] + 10;
+              skip = TRUE;
+            }
+          else if (subcommand_name == NULL)
+            {
+              g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                           "Unknown or invalid admin option: %s", argv[in]);
+              goto out;
+            }
+        }
+
+      /* The global short options */
+      else
+        {
+          skip = FALSE;
+          for (i = 1; argv[in][i] != '\0'; i++)
+            {
+              switch (argv[in][i])
+              {
+                case 'h':
+                  want_help = TRUE;
+                  break;
+
+                default:
+                  if (subcommand_name == NULL)
+                    {
+                      g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                                   "Unknown or invalid admin option: %s", argv[in]);
+                      goto out;
+                    }
+                  break;
+              }
+            }
+        }
+
+      /* Skipping this argument? */
+      if (skip)
+        out--;
+      else
+        argv[out] = argv[in];
     }
-  else if (argc <= 1 || g_str_has_prefix (argv[1], "--help"))
+
+  argc = out;
+
+  if (subcommand_name == NULL || want_help)
     {
       subcommand = admin_subcommands;
       g_print ("usage: ostree admin --sysroot=PATH COMMAND [options]\n");
@@ -76,11 +152,9 @@ ostree_builtin_admin (int argc, char **argv, GFile *repo_path, GCancellable *can
           g_print ("  %s\n", subcommand->name);
           subcommand++;
         }
-      return argc <= 1 ? 1 : 0;
+      return subcommand_name == NULL ? 1 : 0;
     }
 
-  subcommand_name = argv[1];
-
   subcommand = admin_subcommands;
   while (subcommand->name)
     {
@@ -92,14 +166,12 @@ ostree_builtin_admin (int argc, char **argv, GFile *repo_path, GCancellable *can
   if (!subcommand->name)
     {
       g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED,
-                   "Unknown command '%s'", subcommand_name);
+                   "Unknown admin command '%s'", subcommand_name);
       goto out;
     }
 
-  ostree_prep_builtin_argv (subcommand_name, argc-2, argv+2, &subcmd_argc, &subcmd_argv);
-
   sysroot = g_file_new_for_path (opt_sysroot);
-  if (!subcommand->fn (subcmd_argc, subcmd_argv, sysroot, cancellable, error))
+  if (!subcommand->fn (argc, argv, sysroot, cancellable, error))
     goto out;
  
   ret = TRUE;

diff --git a/src/ostree/ot-main.c b/src/ostree/ot-main.c
index 243dfda171690b6f0b0d9009fb6fa76394db8921..4691bce247dd39b57dd596fab9a66753c49b5c41 100644 (file)
--- a/src/ostree/ot-main.c
+++ b/src/ostree/ot-main.c
@@ -55,26 +55,6 @@ ostree_usage (char **argv,
   return (is_error ? 1 : 0);
 }
 
-void
-ostree_prep_builtin_argv (const char  *builtin,
-                          int          argc,
-                          char       **argv,
-                          int         *out_argc,
-                          char      ***out_argv)
-{
-  int i;
-  char **cmd_argv;
-  
-  cmd_argv = g_new0 (char *, argc + 2);
-  
-  cmd_argv[0] = (char*)builtin;
-  for (i = 0; i < argc; i++)
-    cmd_argv[i+1] = argv[i];
-  cmd_argv[i+1] = NULL;
-  *out_argc = argc+1;
-  *out_argv = cmd_argv;
-}
-
 int
 ostree_run (int    argc,
             char **argv,
@@ -84,14 +64,13 @@ ostree_run (int    argc,
   OstreeCommand *command;
   GError *error = NULL;
   GCancellable *cancellable = NULL;
-  int cmd_argc;
-  char **cmd_argv = NULL;
-  gboolean have_repo_arg;
   const char *cmd = NULL;
   const char *repo = NULL;
   const char *host_repo_path = "/ostree/repo";
   GFile *repo_file = NULL;
-  int arg_off;
+  gboolean want_help = FALSE;
+  gboolean skip;
+  int in, out, i;
 
   /* avoid gvfs (http://bugzilla.gnome.org/show_bug.cgi?id=526454) */
   g_setenv ("GIO_USE_VFS", "local", TRUE);
@@ -103,23 +82,101 @@ ostree_run (int    argc,
   if (argc < 2)
     return ostree_usage (argv, commands, TRUE);
 
-  if (g_str_has_prefix (argv[1], "--version"))
+  /*
+   * Parse the global options. We rearrange the options as
+   * necessary, in order to pass relevant options through
+   * to the commands, but also have them take effect globally.
+   */
+
+  for (in = 1, out = 1; in < argc; in++, out++)
     {
-      g_print ("%s\n  %s\n", PACKAGE_STRING, OSTREE_FEATURES);
-      return 0;
-    }
+      /* The non-option is the command, take it out of the arguments */
+      if (argv[in][0] != '-')
+        {
+          skip = (cmd == NULL);
+          if (cmd == NULL)
+              cmd = argv[in];
+        }
 
-  have_repo_arg = g_str_has_prefix (argv[1], "--repo=");
+      /* The global long options */
+      else if (argv[in][1] == '-')
+        {
+          skip = FALSE;
+
+          if (g_str_equal (argv[in], "--"))
+            {
+              break;
+            }
+          else if (g_str_equal (argv[in], "--help"))
+            {
+              want_help = TRUE;
+            }
+          else if (g_str_equal (argv[in], "--repo") && in + 1 < argc)
+            {
+              repo = argv[in + 1];
+              skip = TRUE;
+              in++;
+            }
+          else if (g_str_has_prefix (argv[in], "--repo="))
+            {
+              repo = argv[in] + 7;
+              skip = TRUE;
+            }
+          else if (cmd == NULL && g_str_equal (argv[in], "--version"))
+            {
+              g_print ("%s\n  %s\n", PACKAGE_STRING, OSTREE_FEATURES);
+              return 0;
+            }
+          else if (cmd == NULL)
+            {
+              g_set_error (&error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                           "Unknown or invalid global option: %s", argv[in]);
+              goto out;
+            }
+        }
 
-  if (!have_repo_arg)
-    {
-      arg_off = 2;
-      cmd = argv[arg_off-1];
+      /* The global short options */
+      else
+        {
+          skip = FALSE;
+          for (i = 1; argv[in][i] != '\0'; i++)
+            {
+              switch (argv[in][i])
+              {
+                case 'h':
+                  want_help = TRUE;
+                  break;
+
+                default:
+                  if (cmd == NULL)
+                    {
+                      g_set_error (&error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                                   "Unknown or invalid global option: %s", argv[in]);
+                      goto out;
+                    }
+                  break;
+              }
+            }
+        }
+
+      /* Skipping this argument? */
+      if (skip)
+        out--;
+      else
+        argv[out] = argv[in];
     }
-  else
+
+  argc = out;
+
+  if (cmd == NULL)
     {
-      arg_off = 3;
-      cmd = argv[arg_off-1];
+      if (!want_help)
+        {
+          g_set_error_literal (&error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                               "No command specified");
+        }
+      ostree_usage (argv, commands, TRUE);
+      goto out;
     }
 
   command = commands;
@@ -139,12 +196,11 @@ ostree_run (int    argc,
 
   g_set_prgname (g_strdup_printf ("ostree %s", cmd));
 
-  if (!(command->flags & OSTREE_BUILTIN_FLAG_NO_REPO))
+  if (repo == NULL && !want_help &&
+      !(command->flags & OSTREE_BUILTIN_FLAG_NO_REPO))
     {
-      if (have_repo_arg)
-        repo = argv[1] + strlen ("--repo=");
-      else if (g_file_test ("objects", G_FILE_TEST_IS_DIR)
-               && g_file_test ("config", G_FILE_TEST_IS_REGULAR))
+      if (g_file_test ("objects", G_FILE_TEST_IS_DIR)
+          && g_file_test ("config", G_FILE_TEST_IS_REGULAR))
         repo = ".";
       else if (g_file_test (host_repo_path, G_FILE_TEST_EXISTS))
         repo = host_repo_path;
@@ -160,13 +216,10 @@ ostree_run (int    argc,
   if (repo)
     repo_file = g_file_new_for_path (repo);
   
-  ostree_prep_builtin_argv (cmd, argc-arg_off, argv+arg_off, &cmd_argc, &cmd_argv);
-
-  if (!command->fn (cmd_argc, cmd_argv, repo_file, cancellable, &error))
+  if (!command->fn (argc, argv, repo_file, cancellable, &error))
     goto out;
 
  out:
-  g_free (cmd_argv);
   g_clear_object (&repo_file);
   if (error)
     {

diff --git a/src/ostree/ot-main.h b/src/ostree/ot-main.h
index b314f8ffb188deb64ea4a9f311eefc01d7182bd5..41adee40659178922eeb1d15520339308c8698ab 100644 (file)
--- a/src/ostree/ot-main.h
+++ b/src/ostree/ot-main.h
@@ -35,13 +35,6 @@ typedef struct {
   int flags; /* OstreeBuiltinFlags */
 } OstreeCommand;
 
-void
-ostree_prep_builtin_argv (const char  *builtin,
-                          int          argc,
-                          char       **argv,
-                          int         *out_argc,
-                          char      ***out_argv);
-
 int ostree_main (int    argc, char **argv, OstreeCommand  *commands);
 
 int ostree_run (int argc, char **argv, OstreeCommand *commands, GError **error);